Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improvements for the VPN client #792

Merged
merged 7 commits into from
Oct 14, 2021
Merged

Improvements for the VPN client #792

merged 7 commits into from
Oct 14, 2021

Conversation

Senyoret1
Copy link
Contributor

Did you run make format && make check?
The go code was not changed. npm run lint and npm run build were used.

Changes:

  • Now the sevcer list is populated with data obtained from the discovery service. All columns not returned by the discovery service were removed from the UI. The code related to the removed columns and not longer used was not removed, but commented, just in case it is needed in the future.

  • A bug which made the paginator of the server list page work incorrectly was solved.

  • Now the countries are sorted by country name, not by country code. The previous method had problems with some countries and the list appeared to be wrongly sorted.

  • Now the correct value of the killswitch property is shown. Related to Show the correct app arguments in the UI #720

How to test this PR:
For checking the server list, use a web browser to open {SkywireManagerUrl}/#/vpn/{visorPk}/servers/public/1. That will show the list with the servers obtained from the discovery service. Use the paginator at the bottom of the page and check if it works well. Also, pressing the flag icon at the top of the country column should order the server by country name.

For checking the value of the killswitch property, go to the settings tab.

NOTE: this PR replaces #782

@Senyoret1
Copy link
Contributor Author

The las commit includes the following changes:

  • Now it is possible to configure the minimum number of hops in the desktop VPN client.

@Senyoret1
Copy link
Contributor Author

The last commit includes the following changes:

  • The procedure for determining which UI elements should be shown in the status tab was improved.
  • The connection time label was removed, as there is not way to get the value using the API or a reliable way to calculate it locally.
  • Now the status page has a line with a color that represents the current status of the connection, like in the Android client. This helps to make the UI easier to underestand.
  • Now there is an option for changing the password for connecting to the server.
  • The code for removing the animation in the desconnection button was improved.
  • The text used as title in the connection status page is now in a translatable var, as it should.
  • Now the date column in the history list is shown correctly.

@Senyoret1 Senyoret1 mentioned this pull request Sep 25, 2021
Closed
@Senyoret1 Senyoret1 changed the title [WIP] Improvements for the VPN client Improvements for the VPN client Sep 25, 2021
jdknives
jdknives requested changes Sep 29, 2021
View reviewed changes
Copy link
Member

@jdknives jdknives left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried to test this PR and ran into two problems:

  1. The server list loads but does not display anything. Either we should show an error or an empty list if there are no servers.

  2. The IP detection and country detection does not work for me consistently.

Let me know what info you need if you cant reproduce the issue.

I was using the test deployment which you can use by generating config with -t flag if thats relevant.

@jdknives
Copy link
Member

@Senyoret1 additionally, once the IP/country detection works, can we select a random server from the same country if user connect without having selected s specific server first?

@Senyoret1
Copy link
Contributor Author

The last commit includes the following changes:

  • The discovery service URL was updated.
  • A bug that prevented the VPN-UI to show the servers returned from the discovery service if there are servers without geo information was fixed.
  • The procedure used by the VPN-UI for creating a transport automatically to connect to the server when stating the app was removed.
  • The state check procedure for stoping the app is now more permisive, as it was conflicting with the way in which the vpn-client app status was being reported in some special cases.

@Senyoret1
Copy link
Contributor Author

The last commit includes the following changes:

  • The code for getting the IP and the country was improved, so it is more reliable now.
  • Now the login page and all related procedures work well with the VPN UI when the user authorization options are activated in the hypervisor.

@Senyoret1
Copy link
Contributor Author

The last commit includes the following changes:

  • The code was changed for working well with the changes made to the status reporting made in the back-end.
  • Now the errors are shown in the UI.

@jdknives jdknives merged commit aa85e1d into skycoin:develop Oct 14, 2021
@Senyoret1 Senyoret1 mentioned this pull request Oct 15, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdknives jdknives jdknives requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Senyoret1 @jdknives